Just added the STAINLESS_API_KEY key to the repo :)

I'm moving this one back to draft until I have a great documented solution to share

Parts 1, 2, 3, and 4 of https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/ are important reading if we need to find a way to pass a secret into something triggered on every pull request.

Thanks @bbrowning. I actually already read those articles, unfortunately there are a few details that are making the mentioned solutions a no-go for this repo.
However I have a great solution that won't require any CI secret. I expect to have it ready later today and will move the PR out of draft asap :)

edit: well, not exactly today, my plane internet is spotty

Currently blocked by stainless-api/upload-openapi-spec-action#133. Once merged I will update the github action SHA and that should be ready.

Some notes that may be of interest that I shared on Discord.

The workflow

1. external contributor (EC) forks the repo
2. EC does some modifications to the Stainless config file or your spec file
3. EC opens a PR
4. the CI workflow is triggered, run the github action
5. action creates a new preview branch for your Stainless project, uploads the config + spec file
6. action starts a build (meaning all sdks configured in your config file will be generated + pushed to an internal repo)
7. action wait for the build to complete
8. action reports any build diagnostics as github comment to the PR
9. when the PR is merged, the action start a new build but this time for your main branch, which will be pushed to your public SDK repos

On security

• instead of a static API key in a repo secret the github action will be using github OpenID Connect (OIDC)
  • in short, github mints a short lived token that cannot be tempered with (cryptographically signed)
• the token includes info regarding the repository, that makes it possible to validate you're actually allowed to push to the Stainless project. The only requirement is to have our github app installed in your github org (it does not need to be given read or write access to the repo where you generate your spec)
• to support PRs from forks the workflow needs to rely on the github event pull_request_target , followed by a git checkout to the PR branch
• workflow files aren't at risk, in the case of a fork PR the workflows from the base repo are used. Github will never run workflows maliciously edited by a fork, unless they have been merged
• the github action doesn't evaluate anything from the fork repo, it only read the spec + config file then interact with the Stainless API

@dgellow how could we do a test-run of this? We have a change from @raghotham which updated the types. See #3948

@ashwinb A test run would require the PR to be first merged, then a PR to be opened or rebased/updated to trigger CI checks. At least for PRs from forks, Github only run workflows if they already exist in the base repo.

Edit: actually, given that their PR has already been merged, we would need to open a new one. The workflow I'm adding is triggered on PR events

Hmm - the security guidance from stainless-api/upload-openapi-spec-action says, If your GitHub repository is public, require approval for workflows from fork PRs to prevent untrusted contributors from accessing OIDC tokens or secrets.

We do not require approval for workflows from fork PRs for every untrusted contributor. We only require approval on the first contribution, I believe?

I wouldn't be comfortable merging this, as the usage of pull_request_target combined with cloning the changes from the fork in question means we're very close to exfiltrating or exposing the write tokens given to this job. We have to trust there is nothing being done in that stainless action that could potentially run any of the untrusted code we just cloned.

The wording for that section of the README is a bit too strong (I did write it, I will take another pass to clarify the actual risk).

I would invite you to review the action implementation, it doesn't execute anything from the repo context, it only reads the spec + config files, pass them to the Stainless API, then interact with GitHub API to add a comment.

As mentioned here the use of pull_request_target + cloning the fork repo in that context should be safe.

What would be problematic:

• if a fork PR updates the workflow in a way that would leak the OIDC token, and get it merged. That should be caught by reviewers
• if the stainless github action is updated in a way that would execute some code from the repo (something we see as a pretty massive security issue on our end, it's not something we would introduce willingly)

We discussed this on Discord a bit.

I share @bbrowning's concerns -- I have been bit by pull_request_target earlier (re: the pre-commit workflow). The trouble is that a write token when leaked to a third-party action opens up a much larger surface area (e.g., what if a vulnerability lands into stainless itself?) which is beyond our control. Even within our own fold, it can be hard to guard against vulnerabilities so it is best we don't open up this. If this means that we have to use pull_request and if that in turn means that we cannot get this executed on forks, then we will take that trade-off for now. We have a set of maintainers who have write access to the repo and API updates can just go via these folks for the moment. cc @leseb -- this is of course not ideal, but is it okay in the interim? Because right now, we don't even have this facility and we have to go via the Stainless Editor which is not ideal.

To summarize, here is what the proposed process for making an API change would be:

• One of the maintainers makes the required API changes with the PR being opened in the upstream llamastack/llama_stack repository.
• The "preview" action runs for the PR -- no "write" token is needed, but just an OIDC token with "read" permissions (correct me here @dgellow if I am wrong). The result of the action and the preview build lands as a comment on the PR. This is all kosher.
• Once the PR lands, the stainless action will run another build and if all works out we will see a "next" branch being updated on the client-SDks. A PR comment can be made accordingly as such.
• Finally, we need to update the client-sdk -- this means landing the "stainless release PR" which is associated with the next branch. This action can be done by the workflow directly on our end by invoking an action with write permissions on the corresponding client sdk repositories.

Thoughts here @cdoern @mattf @leseb @bbrowning @ehhuang ?

The "preview" action runs for the PR -- no "write" token is needed, but just an OIDC token with "read" permissions (correct me here @dgellow if I am wrong). The result of the action and the preview build lands as a comment on the PR. This is all kosher.

Yes that's correct. The only role of the OIDC token here is to check the request originates from a repo that has access to the stainless org/project. It's just a more convenient and safer (because short-lived) alternative to adding a Stainless API key to your repo secrets.

The only API writes are:

• on the Stainless API side, to create new builds (and preview stainless branches)
• and updating the comments in the PR

Edit: I forgot to add that as far as I'm aware github OIDC tokens are only used for authentication with external services, they do not provide write permissions to the repo itself! The only risk is regarding Stainless writes, not GitHub writes.

permissions:
  id-token: write # this allows to mint OIDC tokens, but the generated token cannot
                  # be used against the GitHub API.

One tiny detail here:

This action can be done by the workflow directly on our end by invoking an action with write permissions on the corresponding client sdk repositories.

If someone with write access to the repo approves the release PRs in SDK repositories the github app will attempt to merge it, you shouldn't have to run a workflow (but you of course can if you prefer to do it that way). That can be automated easily using the github CLI if you want to do it across all sdk repos

After discussion with @ashwinb in Discord we decided to keep pull_request_target but use the API key instead of OIDC. That way even if the stainless action becomes malicious there is no risk of impersonating the llamastack identity (other than against Stainless).